Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add kaniko service account to image builder configs #357

Merged

Conversation

deadlycoconuts
Copy link
Contributor

@deadlycoconuts deadlycoconuts commented Aug 10, 2023

Context

Similar to how caraml-dev/merlin#352 introduced a Kubernetes service account to be used for Kaniko jobs orchestrated by Merlin, this PR similarly introduces a similar change for Turing in its API server so that it is able to pass a configured Kaniko service account name to all the Kaniko jobs it orchestrates.

Note that not a lot of changes (if any) were made to the unit tests, despite the addition of a new service account (name) field, unlike in the Merlin repo. This is normal since Merlin is currently testing the image job creation workflows much more extensively in its unit tests, which isn't the case in Turing (we are only ascertaining that no error gets returned and the correct container image name gets returned). While it would be nice to introduce more extensive unit tests for Turing (so as to exhaustively test all the transformations performed in order to generate a K8s job manifest), this is out of scope of this PR and will probably be taken on at a later time.

Copy link
Collaborator

@krithika369 krithika369 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one comment - the rest LGTM. Thanks, @deadlycoconuts !

api/turing/imagebuilder/imagebuilder.go Show resolved Hide resolved
@deadlycoconuts deadlycoconuts merged commit b55e9fe into caraml-dev:main Aug 15, 2023
12 checks passed
@deadlycoconuts deadlycoconuts deleted the enable_wi_for_kaniko_jobs branch August 15, 2023 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants